fix(verses-api): support multi-verse tafsirs in verses endpoint & Fix eager loading for chapter reciters #718
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit fixes the issue where multi-verse tafsirs (such as ID 169) were not being returned by the /verses endpoint. Key change: - Remove the `one_verse` constraint from the verses presenter, allowing the endpoint to include tafsirs that span multiple verses. This change enables tafsir resources like Ibn Kathir (ID 169) to be properly included in API responses.
via verse ID range join - Introduced support for multi-ayah tafsir loading in VerseFinder - Added load_tafsirs method to join tafsirs based on verse ID range (start_verse_id to end_verse_id) - Ensured single-ayah tafsir continues to work as expected - Maintains backward compatibility while enabling richer tafsir rendering across APIs like /v4/verses/by_key
…with-eager_load Fix eager loading for chapter reciters
Fix eager loading for chapter reciters
basit3407
changed the title
hammadxcm
reviewed
Aug 12, 2025
There was a problem hiding this comment.
🚨 Major Issues in This PR
1. Missing Test Coverage for Tafsir Loading
- No tests were added for the new
load_tafsirsmethod. - Must add coverage for:
- Single tafsir ID param.
- Multiple tafsir IDs (comma-separated).
- Multi-verse tafsir ranges.
- Ensuring no duplicate verses in results.
- Without this, the new feature could break silently.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enables support for multi-verse tafsirs in the
/verses/by_keyendpoint.🔧 Fix Summary:
.one_verseconstraint fromVersesPresentertafsir filtering logic.load_tafsirsmethod inVerseFinder, which joins tafsirs by checking whether the verse'sidfalls betweenstart_verse_idandend_verse_id.2:1–2:5).params[:tafsirs], avoiding overhead on unrelated endpoints.Summary for list chapters API
✅ Implementation Notes:
/v4/verses/by_key/2:2?tafsirs=169) without verifying multi-verse coverage due to absence of full DB dump.start_verse_idandend_verse_idare populated in production/staging.